Effect persistence layer by juliusmarminge · Pull Request #83 · pingdotgg/t3code

juliusmarminge · 2026-02-20T12:56:34Z

This pull request contains changes generated by a Cursor Cloud Agent

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

cursor · 2026-02-20T12:56:35Z

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
_{Learn more about Cursor Agents}

coderabbitai · 2026-02-20T12:56:43Z

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch cursor/effect-persistence-layer-41a2

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

macroscopeapp · 2026-02-20T12:57:31Z

Adopt an Effect-based SQLite persistence layer for server state and route all CRUD and event flow through `PersistenceService` in persistenceService.ts with async queues and migrations

Introduce an Effect-backed SQLite stack with runtime driver selection, migrations, and schema; refactor server persistence to repositories and domain utils; shift state and provider event handling to internal queues and workers; tighten state event schemas to a discriminated union; and update the web store reducer to consume typed payloads.

📍Where to Start

Start with the constructor and transaction/event flow in PersistenceService in persistenceService.ts, then review migration setup in migrator.ts and the state event schema in state.ts.

📊 Macroscope summarized 40f9885. 24 files reviewed, 65 issues evaluated, 0 issues filtered, 10 comments posted. View details

macroscopeapp · 2026-02-20T13:04:30Z

+import path from "node:path";
+
+export function normalizeCwd(rawCwd: string): string {
+  const resolved = path.resolve(rawCwd.trim());


🟡 Medium

domain/projects.ts:5 On POSIX, directory names can contain leading/trailing whitespace (e.g., "repo "), so .trim() may corrupt valid paths. If this is intentional input sanitization, consider documenting that assumption; otherwise, consider removing .trim().

Suggested change

const resolved = path.resolve(rawCwd.trim());

const resolved = path.resolve(rawCwd);

🚀 Want me to fix this? Reply ex: "fix it for me".

🤖 Prompt for AI

In file apps/server/src/persistence/domain/projects.ts around line 5: On POSIX, directory names can contain leading/trailing whitespace (e.g., `"repo "`), so `.trim()` may corrupt valid paths. If this is intentional input sanitization, consider documenting that assumption; otherwise, consider removing `.trim()`. Evidence trail: apps/server/src/persistence/domain/projects.ts line 5 at commit 40f9885dceed14d5af286fb33d23b48e7fd3e974 shows: `const resolved = path.resolve(rawCwd.trim());` - confirming `.trim()` is called on the path input. POSIX filesystem specification allows whitespace characters in filenames/directory names.

macroscopeapp · 2026-02-20T13:04:30Z

+  return [...new Set(runningTerminalIds)]
+    .map((id) => id.trim())
+    .filter((id) => id.length > 0 && validTerminalIds.has(id))


🟡 Medium

domain/threads.ts:25 Deduplication via Set happens before trim(), so "a" and " a" both survive deduplication and become duplicate "a" entries after trimming. Consider trimming before deduplicating, similar to normalizeTerminalIds.

Suggested change

return [...new Set(runningTerminalIds)]

.map((id) => id.trim())

.filter((id) => id.length > 0 && validTerminalIds.has(id))

return [...new Set(runningTerminalIds.map((id) => id.trim()).filter((id) => id.length > 0))]

.filter((id) => validTerminalIds.has(id))

🚀 Want me to fix this? Reply ex: "fix it for me".

🤖 Prompt for AI

In file apps/server/src/persistence/domain/threads.ts around lines 25-27: Deduplication via `Set` happens before `trim()`, so `"a"` and `" a"` both survive deduplication and become duplicate `"a"` entries after trimming. Consider trimming before deduplicating, similar to `normalizeTerminalIds`. Evidence trail: apps/server/src/persistence/domain/threads.ts lines 24-28 (commit 40f9885): `return [...new Set(runningTerminalIds)].map((id) => id.trim())...` shows Set deduplication before trim(). apps/server/src/persistence/domain/threads.ts lines 7-8 (commit 40f9885): `...new Set(ids.map((id) => id.trim()).filter...)` shows normalizeTerminalIds correctly trims before deduplicating.

macroscopeapp · 2026-02-20T13:04:30Z

  private readonly sessionThreadIds = new Map<string, string>();
  private readonly runtimeThreadIds = new Map<string, string>();
+  private readonly stateEventsQueue = Effect.runSync(Queue.unbounded<StateEvent>());
+  private readonly stateEventsBridge = Effect.runFork(this.runStateEventsBridge());


🟡 Medium

src/persistenceService.ts:195 The stateEventsBridge fiber is started in a field initializer before the constructor body runs. If runPersistenceMigrations throws, the fiber is never interrupted because the caller never receives an instance to call close(). Consider moving fiber creation after migration succeeds, or interrupting the fiber in the catch block.

🚀 Want me to fix this? Reply ex: "fix it for me".

🤖 Prompt for AI

In file apps/server/src/persistenceService.ts around line 195: The `stateEventsBridge` fiber is started in a field initializer before the constructor body runs. If `runPersistenceMigrations` throws, the fiber is never interrupted because the caller never receives an instance to call `close()`. Consider moving fiber creation after migration succeeds, or interrupting the fiber in the `catch` block. Evidence trail: apps/server/src/persistenceService.ts lines 194-235 at commit 40f9885dceed14d5af286fb33d23b48e7fd3e974: - Line 195: `private readonly stateEventsBridge = Effect.runFork(this.runStateEventsBridge());` (field initializer starts fiber) - Lines 198-217: constructor body, with try-catch around `runPersistenceMigrations` at lines 202-213 - Lines 204-213: catch block closes db but does not interrupt stateEventsBridge - Lines 219-235: close() method interrupts fiber at line 230, but caller never gets instance if constructor throws

macroscopeapp · 2026-02-20T13:04:30Z

+    Effect.runSync(Scope.close(this.scope, Exit.void));
+  }
+
+  runWithSqlClient<A, E>(effect: Effect.Effect<A, E, SqlClient.SqlClient>): A {


🟢 Low

persistence/sqliteAdapter.ts:150 runWithSqlClient uses Effect.runSync internally, which throws on async effects. Consider documenting this sync-only restriction in the method signature (e.g., rename to runSyncWithSqlClient) or updating the implementation to handle async effects.

🚀 Want me to fix this? Reply ex: "fix it for me".

🤖 Prompt for AI

In file apps/server/src/persistence/sqliteAdapter.ts around line 150: `runWithSqlClient` uses `Effect.runSync` internally, which throws on async effects. Consider documenting this sync-only restriction in the method signature (e.g., rename to `runSyncWithSqlClient`) or updating the implementation to handle async effects. Evidence trail: apps/server/src/persistence/sqliteAdapter.ts lines 150-152 (runWithSqlClient calls this.runEffect), lines 173-176 (runEffect uses Effect.runSync on line 175) at commit 40f9885dceed14d5af286fb33d23b48e7fd3e974

macroscopeapp · 2026-02-20T13:04:30Z

+        [afterSeq],
+      )
+      .unprepared) as StateEventRow[];
+    return decodeStateEventRows(rows).map((row) =>


🟠 High

repos/stateEventsRepo.ts:93 StateEventRowSchema expects seq: Number and payload_json: String, but the database may return bigint for sequences and NULL for payloads (when JSON.stringify(undefined) is inserted). Consider updating the schema to handle bigint (via Schema.Union) and nullable strings.

🚀 Want me to fix this? Reply ex: "fix it for me".

🤖 Prompt for AI

In file apps/server/src/persistence/repos/stateEventsRepo.ts around line 93: `StateEventRowSchema` expects `seq: Number` and `payload_json: String`, but the database may return `bigint` for sequences and `NULL` for payloads (when `JSON.stringify(undefined)` is inserted). Consider updating the schema to handle `bigint` (via `Schema.Union`) and nullable strings. Evidence trail: apps/server/src/persistence/schema.ts:17 - `NumberOrBigIntSchema = Schema.Union([Schema.Number, Schema.BigInt])` apps/server/src/persistence/schema.ts:34-40 - `StateEventRowSchema` with `seq: Schema.Number` and `payload_json: Schema.String` apps/server/src/persistence/schema.ts:42-44 - `StateSeqRowSchema` uses `Schema.optional(Schema.NullOr(NumberOrBigIntSchema))` for seq apps/server/src/persistence/schema.ts:24-27 - `CompletedProviderItemRowSchema` uses `Schema.NullOr(Schema.String)` for payload_json apps/server/src/persistence/repos/stateEventsRepo.ts:41-52 - `appendStateEvent` with `payload: unknown` and `JSON.stringify(input.payload)` apps/server/src/persistence/repos/stateEventsRepo.ts:93 - `decodeStateEventRows(rows)` usage

macroscopeapp · 2026-02-20T13:04:30Z

+  const normalized = diff.replace(/\r\n/g, "\n");
+  const bPath = normalized.match(/^\+\+\+ b\/(.+)$/m);
+  if (bPath?.[1]) return bPath[1];
+  const gitHeader = normalized.match(/^diff --git a\/(.+) b\/\1$/m);


🟡 Medium

domain/turnSummaries.ts:8 Suggestion: make parsePathFromDiff resilient to renames and deletions. Capture the destination path from the diff --git header (avoid the backreference), and when +++ is /dev/null, derive the path from the --- a/... line so deleted files aren’t omitted.

Suggested change

const gitHeader = normalized.match(/^diff --git a\/(.+) b\/\1$/m);

const gitHeader = normalized.match(/^diff --git a\/.+ b\/(.+)$/m);

🚀 Want me to fix this? Reply ex: "fix it for me".

🤖 Prompt for AI

In file apps/server/src/persistence/domain/turnSummaries.ts around line 8: Suggestion: make `parsePathFromDiff` resilient to renames and deletions. Capture the destination path from the `diff --git` header (avoid the backreference), and when `+++` is `/dev/null`, derive the path from the `--- a/...` line so deleted files aren’t omitted. Evidence trail: apps/server/src/persistence/domain/turnSummaries.ts lines 4-15 at commit 40f9885dceed14d5af286fb33d23b48e7fd3e974. Line 8 shows backreference `\1` in regex `/^diff --git a\/(.+) b\/\1$/m` which prevents rename matching. Lines 10-13 show that when `+++ /dev/null` is detected, the function returns `null` instead of deriving path from `--- a/...` line.

macroscopeapp · 2026-02-20T13:04:30Z

+export function inferProjectName(cwd: string): string {
+  const name = path.basename(cwd);
+  return name.length > 0 ? name : "project";
+}


🟢 Low

domain/projects.ts:21 On Windows, path.basename returns empty for drive roots like D:\ or E:\, so all root paths get the same name "project". Consider extracting the drive letter (e.g., "D-root") to avoid collisions when multiple drive roots are registered.

Suggested change

export function inferProjectName(cwd: string): string {

const name = path.basename(cwd);

return name.length > 0 ? name : "project";

}

export function inferProjectName(cwd: string): string {

const name = path.basename(cwd);

if (name.length > 0) {

return name;

}

// Handle Windows drive roots (e.g., "C:\" -> "C-root")

if (process.platform === "win32") {

const drive = path.parse(cwd).root.replace(/[:\\]/g, "").toUpperCase();

if (drive) {

return `${drive}-root`;

}

}

return "project";

}

🚀 Want me to fix this? Reply ex: "fix it for me".

🤖 Prompt for AI

In file apps/server/src/persistence/domain/projects.ts around lines 21-24: On Windows, `path.basename` returns empty for drive roots like `D:\` or `E:\`, so all root paths get the same name `"project"`. Consider extracting the drive letter (e.g., `"D-root"`) to avoid collisions when multiple drive roots are registered. Evidence trail: apps/server/src/persistence/domain/projects.ts lines 21-24 at commit 40f9885dceed14d5af286fb33d23b48e7fd3e974. The `inferProjectName` function uses `path.basename(cwd)` and falls back to `"project"` when the result is empty. Node.js `path.basename` behavior for Windows drive roots like `D:\` returns empty string (documented behavior in Node.js path module).

macroscopeapp · 2026-02-20T13:04:30Z

+      continue;
+    }
+
+    if (char === ";" && !inSingleQuote && !inDoubleQuote && !inBacktick) {


🟡 Medium

persistence/sqliteAdapter.ts:72 Semicolons inside SQL comments (-- or /* */) will incorrectly split statements, causing syntax errors. Consider adding comment detection to skip semicolons within -- line comments and /* */ block comments.

🚀 Want me to fix this? Reply ex: "fix it for me".

🤖 Prompt for AI

In file apps/server/src/persistence/sqliteAdapter.ts around line 72: Semicolons inside SQL comments (`--` or `/* */`) will incorrectly split statements, causing syntax errors. Consider adding comment detection to skip semicolons within `--` line comments and `/* */` block comments. Evidence trail: apps/server/src/persistence/sqliteAdapter.ts lines 47-90 at commit 40f9885dceed14d5af286fb33d23b48e7fd3e974. Specifically: lines 51-53 show only quote tracking (inSingleQuote, inDoubleQuote, inBacktick), line 72 shows the semicolon split condition only checks these three flags, no comment detection variables or logic exists in the function.

macroscopeapp · 2026-02-20T13:04:30Z

+  if (typeof value === "bigint") {
+    return Number(value);
+  }


🟡 Medium

persistence/sqliteAdapter.ts:96 Converting bigint to number via Number(value) silently loses precision for values exceeding Number.MAX_SAFE_INTEGER. Consider checking bounds and throwing an error, or returning bigint to preserve precision for large lastInsertRowid values.

- if (typeof value === "bigint") { - return Number(value); - } + if (typeof value === "bigint") { + if (value > BigInt(Number.MAX_SAFE_INTEGER) || value < BigInt(Number.MIN_SAFE_INTEGER)) { + throw new RangeError(`Value ${value} exceeds safe integer range`); + } + return Number(value); + }

🚀 Want me to fix this? Reply ex: "fix it for me".

🤖 Prompt for AI

In file apps/server/src/persistence/sqliteAdapter.ts around lines 96-98: Converting `bigint` to `number` via `Number(value)` silently loses precision for values exceeding `Number.MAX_SAFE_INTEGER`. Consider checking bounds and throwing an error, or returning `bigint` to preserve precision for large `lastInsertRowid` values. Evidence trail: apps/server/src/persistence/sqliteAdapter.ts lines 92-100 (toSafeInteger function with bigint->Number conversion), lines 154-167 (runStatement using toSafeInteger for lastInsertRowid). Commit 40f9885dceed14d5af286fb33d23b48e7fd3e974.

cursoragent and others added 22 commits February 20, 2026 09:14

Adopt Effect SQL-backed SQLite adapter

550069f

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

Type state event payloads across contracts and store

e691b4a

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

Run migrations and ingest persistence via Effect runtime

13dea39

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

Use Effect SQL repository for state event persistence

a976a82

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

Extract metadata persistence to Effect SQL repository

56066ce

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

Extract initial schema migration into persistence module

5d75a58

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

Extract document persistence queries into Effect repository

d6e0cd8

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

Move provider event SQL access into Effect repository

bce0df0

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

Document Effect-based persistence architecture

34d456b

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

Route thread message and summary queries through Effect repo

85af69c

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

Move remaining document queries behind Effect repositories

9112818

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

Initialize persistence service directly from Effect SQLite adapter

aa2d6e7

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

Remove legacy StateDb wrapper and obsolete tests

1db852a

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

Extract persistence domain helpers and add turn summary tests

40d49f1

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

Add Effect persistence runtime modules and wire service bootstrap

2cf729e

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

Replace legacy sqlite adapter tests with persistence module tests

ee2211b

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

Remove legacy SQL fallback paths from persistence service

ba0a08f

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

Drop non-Effect migration fallback path

5ba2218

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

Bridge state event publication through Effect queue worker

10d6321

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

Add migration reliability tests for Effect-only adapter requirements

cdea401

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

Move sqlite adapter and migrator fully into persistence module

72c33f7

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

Validate persistence repository rows with Effect Schema decoders

40f9885

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>

macroscopeapp Bot reviewed Feb 20, 2026

View reviewed changes

juliusmarminge closed this Feb 21, 2026

This was referenced Sep 7, 2026

chore(upstream): sync marcode through 8b2838e0 maarco/marcode#76

Draft

chore(upstream): sync marcode through 061543e9 maarco/marcode#78

Draft

Navigation Menu

Uh oh!

Effect persistence layer - #83

Effect persistence layer#83
juliusmarminge wants to merge 22 commits into
codething/48364d50from
cursor/effect-persistence-layer-41a2

juliusmarminge commented Feb 20, 2026

Uh oh!

cursor Bot commented Feb 20, 2026

Uh oh!

coderabbitai Bot commented Feb 20, 2026

Review skipped

Uh oh!

macroscopeapp Bot commented Feb 20, 2026 •

edited

Loading

Uh oh!

macroscopeapp Bot Feb 20, 2026

Uh oh!

macroscopeapp Bot Feb 20, 2026

Uh oh!

macroscopeapp Bot Feb 20, 2026

Uh oh!

macroscopeapp Bot Feb 20, 2026

Uh oh!

macroscopeapp Bot Feb 20, 2026

Uh oh!

macroscopeapp Bot Feb 20, 2026

Uh oh!

Uh oh!

macroscopeapp Bot Feb 20, 2026

Uh oh!

macroscopeapp Bot Feb 20, 2026

Uh oh!

macroscopeapp Bot Feb 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	const resolved = path.resolve(rawCwd.trim());
	const resolved = path.resolve(rawCwd);

	const gitHeader = normalized.match(/^diff --git a\/(.+) b\/\1$/m);
	const gitHeader = normalized.match(/^diff --git a\/.+ b\/(.+)$/m);

-export function inferProjectName(cwd: string): string {
-  const name = path.basename(cwd);
-  return name.length > 0 ? name : "project";
-}
+export function inferProjectName(cwd: string): string {
+  const name = path.basename(cwd);
+  if (name.length > 0) {
+    return name;
+  }
+  // Handle Windows drive roots (e.g., "C:\" -> "C-root")
+  if (process.platform === "win32") {
+    const drive = path.parse(cwd).root.replace(/[:\\]/g, "").toUpperCase();
+    if (drive) {
+      return `${drive}-root`;
+    }
+  }
+  return "project";
+}

Uh oh!

Conversation

juliusmarminge commented Feb 20, 2026

Uh oh!

cursor Bot commented Feb 20, 2026

Uh oh!

coderabbitai Bot commented Feb 20, 2026

Review skipped

Uh oh!

macroscopeapp Bot commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Adopt an Effect-based SQLite persistence layer for server state and route all CRUD and event flow through PersistenceService in persistenceService.ts with async queues and migrations

📍Where to Start

Uh oh!

macroscopeapp Bot Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

macroscopeapp Bot Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

macroscopeapp Bot commented Feb 20, 2026 •

edited

Loading

Adopt an Effect-based SQLite persistence layer for server state and route all CRUD and event flow through `PersistenceService` in persistenceService.ts with async queues and migrations